Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ip6 scoped linklocal #113

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

christophefontaine
Copy link
Contributor

First try to implement scoped link local addresses.

Still has some issues:

grout# add ip6 route ::0/0 via fe80::4e77:6dff:fe7d:1cc7
error: command failed: No route to host

This link local address doesn't exist in the FIB, but fe80:100::4e77:6dff:fe7d:1cc7 and fe80:300::4e77:6dff:fe7d:1cc7 are present internally.

grout# add ip6 route ::0/0 via fe80:100::4e77:6dff:fe7d:1cc7
grout# show ip6 route
VRF  DESTINATION                    NEXT_HOP
0    fe80::209:c000:17e:f6b/128     fe80::209:c000:17e:f6b
0    fe80::4e77:6dff:fe7d:1cc7/128  fe80::4e77:6dff:fe7d:1cc7
0    fe80::1:1/128                  fe80::1:1
0    fe80::4e77:6dff:fe7d:1cc7/128  fe80::4e77:6dff:fe7d:1cc7
0    fe80::90e2:ba00:314:2f8c/128   fe80::90e2:ba00:314:2f8c
0    ::/0                           fe80::4e77:6dff:fe7d:1cc7

We may have a similar linklocal next hop (same ip, same mac) on 2 different interfaces.
In order to support scoped linklocal addresses, we encode the interface id
in the second nibble of the ipv6 address:

"fe80::209:c000:17e:f6b on interface id 1" becomes "fe80:100::209:c000:17e:f6b".

Signed-off-by: Christophe Fontaine <cfontain@redhat.com>
Do not expose the internal encoding of the interface in
the link local address.

Signed-off-by: Christophe Fontaine <cfontain@redhat.com>
@christophefontaine
Copy link
Contributor Author

I'll update the API to allow the client to specify the interface.

Copy link
Collaborator

@rjarry rjarry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines -79 to +97
static int ip6_mcast_addr_add(struct iface *iface, const struct rte_ipv6_addr *ip) {
static int ip6_mcast_addr_add(struct iface *iface, const struct rte_ipv6_addr *ipv6) {
struct hoplist *maddrs = &iface_mcast_addrs[iface->id];
struct nexthop *nh = NULL;
struct rte_ipv6_addr ip = *ipv6;
unsigned i;

LOG(INFO, "%s: joining multicast group " IP6_F, iface->name, ip);
LOG(INFO, "%s: joining multicast group " IP6_F, iface->name, &ip);

for (i = 0; i < maddrs->count; i++)
if (rte_ipv6_addr_eq(&maddrs->nh[i]->ipv6, ip))
if (rte_ipv6_addr_eq(&maddrs->nh[i]->ipv6, &ip))
return errno_set(EEXIST);

if (i == ARRAY_DIM(maddrs->nh))
return errno_set(ENOSPC);

if ((nh = ip6_nexthop_lookup(iface->vrf_id, ip)) == NULL) {
if ((nh = ip6_nexthop_new(iface->vrf_id, GR_IFACE_ID_UNDEF, ip)) == NULL)
if ((nh = ip6_nexthop_lookup(iface->vrf_id, &ip)) == NULL) {
if ((nh = ip6_nexthop_new(iface->vrf_id, GR_IFACE_ID_UNDEF, &ip)) == NULL)
return errno_set(-errno);
rte_ether_mcast_from_ipv6(&nh->lladdr, ip);
rte_ether_mcast_from_ipv6(&nh->lladdr, &ip);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I missed something. Why are you introducing a new local ip variable to use it instead of the const parameter?

struct hoplist *addrs;
unsigned addr_index;
struct rte_ipv6_addr *ip = (struct rte_ipv6_addr *)ipv6;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we need to change the signature of the function we can discuss it, no worries. But please no hacks like this.


if ((ret = iface6_addr_add(iface, &req->addr.addr.ip, req->addr.addr.prefixlen)) < 0)
ip6_addr_linklocal_scope(&ip, req->addr.iface_id);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are adding the iface id scope twice: here and in iface6_addr_add.

Comment on lines +220 to +221
scoped = req->addr.addr.ip;
ip6_addr_linklocal_scope(&scoped, req->addr.iface_id);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is safe to scope the address present in the request. no need for a local variable.

@@ -40,4 +40,18 @@ struct hoplist *ip6_addr_get_all(uint16_t iface_id);
// determine if the given interface is member of the provided multicast address group
struct nexthop *ip6_mcast_get_member(uint16_t iface_id, const struct rte_ipv6_addr *mcast);

static inline int ip6_addr_linklocal_scope(struct rte_ipv6_addr *ip, uint16_t iface_id) {
if (!rte_ipv6_addr_is_linklocal(ip))
return errno_set(EFAULT);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EINVAL would be more appropriate.

@@ -40,4 +40,18 @@ struct hoplist *ip6_addr_get_all(uint16_t iface_id);
// determine if the given interface is member of the provided multicast address group
struct nexthop *ip6_mcast_get_member(uint16_t iface_id, const struct rte_ipv6_addr *mcast);

static inline int ip6_addr_linklocal_scope(struct rte_ipv6_addr *ip, uint16_t iface_id) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not fond of the function name but I don't have great alternatives to suggest either.

static inline int ip6_addr_linklocal_scope(struct rte_ipv6_addr *ip, uint16_t iface_id) {
if (!rte_ipv6_addr_is_linklocal(ip))
return errno_set(EFAULT);
((uint16_t *)ip->a)[1] = iface_id;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be best to change this to network order to ensure consistent behavior across architectures.

rte_be16_t scope_id = htons(iface_id);
// rte_ipv6_addr is aligned to 1, avoid alignment issues with casting to uint16_t
ip->a[2] = 0xff & (scope_id >> 8);
ip->a[3] = 0xff & scope_id;

static inline int ip6_addr_linklocal_unscope(struct rte_ipv6_addr *ip) {
if (!rte_ipv6_addr_is_linklocal(ip))
return errno_set(EFAULT);
((uint16_t *)ip->a)[1] = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// rte_ipv6_addr is aligned to 1, avoid alignment issues with casting to uint16_t
ip->a[2] = 0;
ip->a[3] = 0;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants